Skip to content

Fix two #223 regressions: "-(x+1)**2" sign and "- -x" as "--x" - #225

Merged
sspickle merged 3 commits into
masterfrom
fix/unary-double-minus
Nov 5, 2026
Merged

sspickle merged 3 commits into
masterfrom
fix/unary-double-minus

Conversation

@sspickle

@sspickle sspickle commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes two regressions from #223 (#170). Before #223, the preprocessor's "-" → "-1*" rewrite hid two RapydScript bugs, and removing it exposed both. glowscript.org has been rolled back to the version before #223 (20261101t092705) until this is merged.

1. -(x+1)**2 came out positive

RapydScript's parser gave unary operators higher precedence than **, so -x**2 parsed as (-x)**2. The output stage then turned that back into -Math.pow(x, 2), but only when the unary had no parentheses. Everything else came out with the wrong sign:

Python with #223 this PR
-(x+1)**2 (x = 5) -36 36 -36
-(x)**2 -25 25 -25
-(2)**2 -4 4 -4
- -x**2 25 -25 25
exp(-(x-3)**2/8) 0.61 1.65 0.61

Fix (61d0edf): maybe_unary now parses Python's primary ** factor itself. After an atom, ** takes the next unary expression as its right operand. That makes ** bind tighter than a unary operator on its left, and makes it group right to left. That also fixes two items from #224: 2**3**2 (was 64, now 512) and 2**-1**2 (was 0.25, now 0.5). Reported upstream as kovidgoyal/rapydscript-ng#195; Kovid fixed it the same way in kovidgoyal/rapydscript-ng@50aa052 (Nov 3).

2. - -x compiled to --x

RapydScript printed a unary operator directly against its operand, so - -x became --x. In JavaScript that's a pre-decrement: it gives the wrong value and changes x. - -3 was a syntax error, so the program didn't compile at all.

Fix (b185ec0): print_unary_prefix now adds a space when a sign operator follows another sign operator. Upstream fixed this the same day as kovidgoyal/rapydscript-ng#194 (ec30155).

Testing

🤖 Generated with Claude Code

RapydScript prints a unary operator directly against its operand, so
"- -x" came out as "--x": a pre-decrement that returns x-1 and changes
x, and for a literal ("- -3") a JavaScript syntax error that stops the
whole program from compiling. "+ +x" became "++x" the same way.

Before #170 the preprocessor rewrote every "-" as "-1*", which hid
this. Now print_unary_prefix adds a space when a sign operator is
followed by another sign operator (UglifyJS's rule for the same case).
Plain "-x" still compiles to "-x".

tests/test_compiler.py compiles and runs ten repeated-sign expressions
under node and compares them with CPython. Before the fix four fail
("- -3", "- - -x" syntax errors; "- -x" == 4, "+ +x" == 6, x changed);
after it all pass. Through GlowScriptOffline, a 122-row arithmetic
program matches CPython except for the 15 known pre-existing
differences, the #170 39-case program has 0 failures, and all 23
bundled demos run without errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QNw6KLiif2r9zSoSQug4iL
@sspickle

sspickle commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

On hold. #223 (#170) has a second, larger regression, and this fix doesn't cover it.

Unary minus in front of a parenthesized base of ** now has the wrong sign:

Python Python gives with #223
-(x+1)**2 (x = 5) -36 36
-(x)**2 -25 25
-(2)**2 -4 4
- -x**2 25 -25
exp(-(x-3)**2/8) 0.61 1.65

Cause: RapydScript's parser binds a unary operator tighter than **. The output stage then repairs only an unparenthesized -x**2. The old "-" → "-1*" rewrite was hiding this. Every build before #223 gets all of these right.

glowscript.org has been rolled back to the version before #223 (20261101t092705). A parser fix, so that ** binds tighter than a unary minus on its left and groups right to left as in Python, will follow, with tests, before this is merged.

🤖 Generated with Claude Code

@sspickle
sspickle marked this pull request as draft November 3, 2026 12:39
RapydScript gave unary operators the highest precedence, so "-x**2"
parsed as "(-x)**2", and the output stage turned that back into
"-Math.pow(x, 2)" only when the unary had no parentheses. Everything
else came out with the wrong sign:

    -(x+1)**2  -> 36    (Python: -36)
    -(x)**2    -> 25    (Python: -25)
    - -x**2    -> -25   (Python: 25)
    exp(-(x-3)**2/8) -> 1.65  (Python: 0.61)

The "-" -> "-1*" rewrite removed in #223 hid all of this, so #223
made it visible. maybe_unary now parses Python's "primary ** factor"
itself: after an atom, "**" takes the next unary expression as its
right operand. That binds ** tighter than a unary on its left and makes
it group right to left, which also fixes 2**3**2 (was 64, now 512) and
2**-1**2 (was 0.25, now 0.5) from #224.

tests/test_compiler.py: 21 power/unary expressions against CPython; 9
failed before the fix, all pass after. Through GlowScriptOffline: the
122-row arithmetic table now differs from CPython only in the 13 known
cases (% with negatives, round halves, int(bool), e), the #170 39-case
program has 0 failures, and all 23 demos run. **kwargs in calls and
definitions, comprehensions and conditionals with ** are unaffected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QNw6KLiif2r9zSoSQug4iL
@sspickle sspickle changed the title Fix "- -x" compiling to the JavaScript decrement "--x" (regression from #223) Fix two #223 regressions: "-(x+1)**2" sign and "- -x" as "--x" Nov 3, 2026
@sspickle
sspickle marked this pull request as ready for review November 4, 2026 00:12
@sspickle

sspickle commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Ready for review again. This now includes the second fix: ** binds tighter than a unary operator on its left and groups right to left, as in Python, so -(x+1)**2 is -36 again and 2**3**2 is 512. Upstream RapydScript fixed it the same way in kovidgoyal/rapydscript-ng@50aa052 (issue kovidgoyal/rapydscript-ng#195).

Tested with rsWVPRunner v3.2.4-rc3 (RScompiler sha256 ac91f82f…) on both trinket trial deploys:

🤖 Generated with Claude Code

"GlowScript 3.2 VPython" programs load package/RScompiler.3.2.min.js, not
lib/rapydscript/compiler.js. The two parser fixes on this branch changed
lib only, so on glowscript.org they would have done nothing. Rebuilt with
build_original_py3.py (Docker, node 20); only RScompiler changed -- glow,
compiler and RSrun came out byte-identical to the committed files. The
GlowScriptOffline copy is updated to match, as #223 did.

tests/test_compiler.py now runs every case against the package too, loaded
into a vm context the way a browser would, so a lib-only fix fails the
test instead of passing silently.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QNw6KLiif2r9zSoSQug4iL
@sspickle

sspickle commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Added one commit (98805c6) after Bruce pointed to docs/MakingNewVersion.txt.

Programs with a GlowScript 3.2 VPython header load package/RScompiler.3.2.min.js, not lib/rapydscript/compiler.js. The two parser fixes here changed only lib, so as first posted this PR would have changed nothing on glowscript.org. Thanks, Bruce.

What the commit does

  • Rebuilds package/RScompiler.3.2.min.js with build_original_py3.py (in Docker, node 20). Only RScompiler changed. glow, compiler and RSrun rebuilt byte-for-byte identical to the committed files, so nothing unrelated moved.
  • Copies it to GlowScriptOffline/glowscript_libraries/, as fix(#170): stop rewriting "-" as "-1*" before compiling #223 did. The two copies were identical before.
  • Keeps the GlowScript customizations from the GScompiler.js instructions (vec ==/!=, .pypop, the old ρσ_list_*/ρσ_in). The fixes were patched into the existing compiler.js rather than replacing it with a fresh RapydScript-NG build, and the counts of each customization in the package are unchanged.
  • Runs tests/test_compiler.py against the package too, loaded in a node vm context the way a browser loads it. Against the old package these tests fail (-(2)**2 gave 4, and - -x**2 gave -25 while changing x). With the rebuilt one all 4 pass, so a fix to lib alone can't pass silently again.

Tests: full suite 27 passed, 4 skipped. The skipped 4 are the compiler tests, because the usual python:slim image has no node. They pass in a node image.

After merge: gcloud app deploy --no-promote, check 3/-3, -2**2 and - -x on the version URL, then migrate traffic.

🤖 Generated with Claude Code

@sspickle
sspickle merged commit f9bf597 into master Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant